Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Template Part Block: Add a block label #24557

Closed
wants to merge 15 commits into from
Closed

Conversation

jeyip
Copy link
Contributor

@jeyip jeyip commented Aug 13, 2020

Description

When the child of a template part block is selected, it can be unclear which template part we're modifying.

Screen Shot 2020-08-12 at 2 18 35 PM

Screen Shot 2020-08-12 at 2 18 42 PM

This PR prototypes template part labels when a child block is selected (as discussed here)

Mocks

Screen Shot 2020-08-11 at 9 59 26 AM

How has this been tested?

  1. Set up the site editor locally
  2. Navigate to the site editor page
  3. Identify a template part block
  4. Select a child of a template part block
  5. Verify that there is a computed label matching the name of the current template part at the top left corner of the block

Screenshots

When selecting a child of a template part block

Screen Shot 2020-08-13 at 3 18 59 PM

Types of changes

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

@github-actions
Copy link

github-actions bot commented Aug 13, 2020

Size Change: +720 B (0%)

Total Size: 1.16 MB

Filename Size Change
build/block-editor/index.js 126 kB +134 B (0%)
build/block-library/editor-rtl.css 8.6 kB +105 B (1%)
build/block-library/editor.css 8.6 kB +103 B (1%)
build/block-library/index.js 134 kB +378 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.67 kB 0 B
build/api-fetch/index.js 3.44 kB 0 B
build/autop/index.js 2.82 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 7.96 kB 0 B
build/block-directory/style-rtl.css 953 B 0 B
build/block-directory/style.css 952 B 0 B
build/block-editor/style-rtl.css 10.7 kB 0 B
build/block-editor/style.css 10.7 kB 0 B
build/block-library/style-rtl.css 7.42 kB 0 B
build/block-library/style.css 7.43 kB 0 B
build/block-library/theme-rtl.css 729 B 0 B
build/block-library/theme.css 730 B 0 B
build/block-serialization-default-parser/index.js 1.88 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 47.7 kB 0 B
build/components/index.js 200 kB 0 B
build/components/style-rtl.css 15.7 kB 0 B
build/components/style.css 15.7 kB 0 B
build/compose/index.js 9.68 kB 0 B
build/core-data/index.js 12.3 kB 0 B
build/data-controls/index.js 1.29 kB 0 B
build/data/index.js 8.55 kB 0 B
build/date/index.js 5.38 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 568 B 0 B
build/dom/index.js 4.47 kB 0 B
build/edit-navigation/index.js 11.5 kB 0 B
build/edit-navigation/style-rtl.css 1.16 kB 0 B
build/edit-navigation/style.css 1.16 kB 0 B
build/edit-post/index.js 304 kB 0 B
build/edit-post/style-rtl.css 5.61 kB 0 B
build/edit-post/style.css 5.61 kB 0 B
build/edit-site/index.js 17 kB 0 B
build/edit-site/style-rtl.css 3.06 kB 0 B
build/edit-site/style.css 3.06 kB 0 B
build/edit-widgets/index.js 11.8 kB 0 B
build/edit-widgets/style-rtl.css 2.45 kB 0 B
build/edit-widgets/style.css 2.45 kB 0 B
build/editor/editor-styles-rtl.css 537 B 0 B
build/editor/editor-styles.css 539 B 0 B
build/editor/index.js 45.3 kB 0 B
build/editor/style-rtl.css 3.8 kB 0 B
build/editor/style.css 3.79 kB 0 B
build/element/index.js 4.65 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 7.71 kB 0 B
build/format-library/style-rtl.css 547 B 0 B
build/format-library/style.css 548 B 0 B
build/hooks/index.js 2.13 kB 0 B
build/html-entities/index.js 621 B 0 B
build/i18n/index.js 3.56 kB 0 B
build/is-shallow-equal/index.js 711 B 0 B
build/keyboard-shortcuts/index.js 2.52 kB 0 B
build/keycodes/index.js 1.94 kB 0 B
build/list-reusable-blocks/index.js 3.11 kB 0 B
build/list-reusable-blocks/style-rtl.css 476 B 0 B
build/list-reusable-blocks/style.css 476 B 0 B
build/media-utils/index.js 5.33 kB 0 B
build/notices/index.js 1.79 kB 0 B
build/nux/index.js 3.4 kB 0 B
build/nux/style-rtl.css 671 B 0 B
build/nux/style.css 668 B 0 B
build/plugins/index.js 2.56 kB 0 B
build/primitives/index.js 1.41 kB 0 B
build/priority-queue/index.js 789 B 0 B
build/redux-routine/index.js 2.85 kB 0 B
build/rich-text/index.js 13.9 kB 0 B
build/server-side-render/index.js 2.77 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.27 kB 0 B
build/url/index.js 4.06 kB 0 B
build/viewport/index.js 1.85 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

@jeyip jeyip force-pushed the try/template-part-labels branch from 501ed86 to 2c93ec4 Compare August 13, 2020 20:31
@jeyip jeyip force-pushed the try/template-part-labels branch from 2c93ec4 to af2ef8c Compare August 13, 2020 21:41
@jeyip jeyip changed the title Add template part label Template Part Block: Add a block label Aug 13, 2020
@@ -0,0 +1,27 @@
/**
Copy link
Contributor Author

@jeyip jeyip Aug 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note:

We can definitely talk about refactoring the labels component for reusability if we move forward with template part labels

@jeyip jeyip self-assigned this Aug 14, 2020
Copy link
Contributor

@Addison-Stavlo Addison-Stavlo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am liking this, its working well! 🎉

Some thoughts on other additions (probably future PRs if design likes this enough as is for now):

  • A hover state that triggers a pop-up/tooltip next to the label that can have text describing how it behaves differently from other blocks.
  • Refactoring this out of template part so we can use it for other blocks with their own custom descriptions.

packages/block-library/src/template-part/editor.scss Outdated Show resolved Hide resolved
packages/block-library/src/template-part/editor.scss Outdated Show resolved Hide resolved
Comment on lines +8 to +13
const [ title ] = useEntityProp(
'postType',
'wp_template_part',
'title',
postId
);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have a strong opinion on this, but figured I would point it out. It seems we are also using this useEntityProp for title in the name-panel component. Maybe it would make sense to move it into index and pass it down to these two child components? But maybe it doesn't really matter and is more simple this way.

Copy link
Contributor Author

@jeyip jeyip Aug 14, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have a strong opinion either. If this becomes widely shared, I'd err on the side of removing implicit dependencies and moving useEntityProp out of the label component. The consuming component can always pass the value through props, and the label component will only be responsible for rendering text.

packages/block-library/src/template-part/edit/index.js Outdated Show resolved Hide resolved
@dubielzyk
Copy link

This is looking nice! Awesome job. Let's tweak the label slightly to make it same as the mocks (https://d.pr/i/DX7mKm).

  • Font-size should be 10px (currently it's 13px)
  • Font-family should be system font (currently it's using Fira Sans)
  • Font-weight should be 500 (not 400)
  • Letter-spacing should be 0.25px
  • The height of the label should be 22px in total (currently it's sitting at 21px. Sorry 😂)
  • The label border should match the selection border, so that's already good. Ignore that in the mock.

We're now left with an interesting issue (which is why I didn't wanna go to the label immediately): The label we added is another label similar to the selected block label. So we effectively have two labels for the same thing.

@Addison-Stavlo
Copy link
Contributor

Addison-Stavlo commented Aug 17, 2020

Font-size should be 10px (currently it's 13px)

@dubielzyk - Im curious about this, why are we going with 10px? He had it at 10px previously and it seemed incredibly small and inconsistent with the rest of the editor. I had recommended the change to $default_font_size (which is the 13px) at some point last week. Looking at the font sizes used in this repo, $editor-font-size is 16px and the smallest font size we seem to have defined the mentioned 13px. Diving into sizes a bit more, it seems there are a handful of places where we use 11px or 12px for different item/title/labels for blocks in the editor, but 10px seems almost unseen (1 usage in a mobile file). Is there a reason why we need to go smaller than all the font sizes used across the editor/repo?

Font-family should be system font (currently it's using Fira Sans)

$default-font should work for this (it defines system font as 1st priority):

$default-font: -apple-system, BlinkMacSystemFont,"Segoe UI", Roboto, Oxygen-Sans, Ubuntu, Cantarell,"Helvetica Neue", sans-serif;

@shaunandrews
Copy link
Contributor

image

The block toolbar of the first child appears on top of the new label.

--

Similarly, in Select mode the new label competes with the existing button:

image

--

image

The new label seems to be picking up some theme styles. The screenshot above is with Twenty Nineteen active.

Also, the icon here doesn't really correlate to the Template Part block icon (the LEGO):

image

@shaunandrews
Copy link
Contributor

Font-size should be 10px (currently it's 13px)
Font-family should be system font (currently it's using Fira Sans)
Font-weight should be 500 (not 400)
Letter-spacing should be 0.25px

I wonder if there is an existing style we can use; 10px seems a little small, and I'm not sure we use letter-spacing elsewhere.

The height of the label should be 22px

Where does this number come from? I struggle myself with the general "grid" in Gutenberg and 22px isn't a value I've come across.

@MichaelArestad
Copy link
Contributor

I just tested this out locally. Overall, the concept works for me. I don't think it's ready just yet as I think some iteration is needed around where the label appears in a variety of situations:

  1. As Shaun pointed out above, the label can be mostly covered in Select mode. Should the label move to another corner in that situation? Should it remain visible at all in select mode?
  2. When template parts are nested, the labels overlap like this:

image

What should be the rules around these labels in situations like this?

Is the icon supposed to be a reusable block icon for template parts?

@jeyip jeyip force-pushed the try/template-part-labels branch 2 times, most recently from 8562c6b to 00ec050 Compare August 17, 2020 17:23
@jeyip jeyip changed the title Template Part Block: Add a block label Template Part Block: Add a block label [Prototype] Aug 17, 2020
@jeyip jeyip added [Status] In Progress Tracking issues with work in progress [Feature] Full Site Editing labels Aug 17, 2020
@jeyip
Copy link
Contributor Author

jeyip commented Aug 17, 2020

We're now left with an interesting issue (which is why I didn't wanna go to the label immediately): The label we added is another label similar to the selected block label. So we effectively have two labels for the same thing.

I see. Which brings us back to the question of why we have two modes in the first place. @dubielzyk 😛

As Shaun pointed out above, the label can be mostly covered in Select mode. Should the label move to another corner in that situation? Should it remain visible at all in select mode?

Great point! I'm removing it entirely in select mode (for the time being), but this is definitely an ongoing discussion. It seems like the selection block labels are genericized with "Group", "Link", "Site Title", so it doesn't make sense to me to display custom names at all.

Also, the icon here doesn't really correlate to the Template Part block icon (the LEGO):

The icon is a placeholder. @shaunandrews @MichaelArestad My mistake for not clarifying in the description of the PR, but these code changes aren't meant to be the finalized version of what we implement. We're in the process of deciding whether or not these labels are best for improving the template part editing user experience, and a lot of details are up for discussion.

More context can be found in this issue #22064

@noahtallen
Copy link
Member

When template parts are nested, the labels overlap like this:

Should we only show the label of the "current active" template part, not the grandparent template part? This makes sense to me, because when you edit the child template part, you are only saving changes to the child. You don't actually modify the grandparent template part from a technical POV.

It seems like the selection block labels are genericized with "Group", "Link", "Site Title", so it doesn't make sense to me to display custom names at all.

I think we've made some changes to this in #23847, where the label in select mode uses the dynamic label of the block (which is something like Template Part: Header now)

Also, the icon here doesn't really correlate to the Template Part block icon

One of the ideas with the icon here is to communicate "reusability" and that the template part is a bit unique in that regard. IMO, it might be possible to find an icon which could to indicate reusability across a wide variety of blocks. Example: how does the user know that their site title block will actually have a very wide affect across the entirety of their site? Perhaps an icon could help indicate that the site title block is special in that way.

Really happy to see some progress here!

@dubielzyk
Copy link

Wonderful comments @shaunandrews and @MichaelArestad . Thanks everyone. I'll work on improvements, but below are a few label alternatives I've looked at. Some definitely have the same issues so ignore those 😂
image

That being said, while the idea of a Label might feel like it belongs more in the Select mode I really enjoy playing with this PR. The label only shows when you're editing a template part and the label is still pretty subtle. I feel like it adds clarity to what you're actually editing.
image

I'm working on some further communication and messaging ideas and one that comes to mind is putting some info in the Block inspector. I'll create a separate issue for this 😃

@MichaelArestad
Copy link
Contributor

That being said, while the idea of a Label might feel like it belongs more in the Select mode I really enjoy playing with this PR.

To clarify, I don't think the label is necessary in select mode. Just edit mode.

@jeyip
Copy link
Contributor Author

jeyip commented Aug 18, 2020

Should we only show the label of the "current active" template part, not the grandparent template part? This makes sense to me, because when you edit the child template part, you are only saving changes to the child. You don't actually modify the grandparent template part from a technical POV.

I'm curious what @shaunandrews and @MichaelArestad think, but this makes a lot of sense to me @noahtallen. I'll try to explore this idea.

@MichaelArestad
Copy link
Contributor

Should we only show the label of the "current active" template part, not the grandparent template part?

@jeyip Showing the label of only the current active template part makes sense to me and further clarifies to the user what they are editing.

@shaunandrews
Copy link
Contributor

The more I think about this, I think adding more "floating UI" into the canvas is likely the wrong approach. I think exposing the current template part in the top bar next to the document title ( Homepage •  Footer ) might be better overall: #20877

@jeyip
Copy link
Contributor Author

jeyip commented Aug 25, 2020

The more I think about this, I think adding more "floating UI" into the canvas is likely the wrong approach. I think exposing the current template part in the top bar next to the document title ( Homepage • Footer ) might be better overall: #20877

@shaunandrews thanks for the input! I like the idea, particularly because the topbar is stickied and always visible, which isn't true for the current mocks for template part labels.

I have some questions though.

  1. It seems like, with the visual distance of the topbar to template part blocks, there might be more friction in associating the topbar label with the template we're editing. Could you elaborate on why "adding more "floating UI" into the canvas is likely the wrong approach"? Is it because it consolidates site-related information in one place?
  2. What would the display for nested template parts look like? Would it be something like (Homepage • Footer • FooterSection)?

@dubielzyk I'm curious what your thoughts are as well.

@shaunandrews
Copy link
Contributor

Could you elaborate on why "adding more "floating UI" into the canvas is likely the wrong approach"?

Any UI we add into the canvas area is likely to affect the display of the document; In this case, the label would obscure other elements. Along the same lines, as the canvas starts to display a more realistic rendering of the front-end of a site, these sorts of floating UI will have to compete with a site's visual design.

I always take pause when I see more UI being introduce into the canvas itself.

@dubielzyk
Copy link

To me adding it to the top bar seems like an unnatural place to show it, but it seems like adding a label to the canvas is going against a lot of principles and isn't getting overall agreement. I'm fine with moving forward with that solution. I'm working on the Document Settings (#20877) so I can have a deeper look at that solution.

Thanks for the feedback and thoughts @shaunandrews and @jeyip :)

@jameskoster
Copy link
Contributor

The more I think about this, I think adding more "floating UI" into the canvas is likely the wrong approach.

+1 on this. I might even go one step further, and question why we're adding this at all. There are already two locations where the template part is exposed to users; the breadcrumb, and the block navigator. Do we need another one? If so, what problem is it solving?

To be clear, I totally appreciate why users need to understand the significance of editing template parts. But it seems to me that the important consequence of that behaviour is the impact it will have on the rest of the site when changes are saved. I'm not sure that displaying a label on the currently focused template part really conveys that, assuming that the average user likely doesn't know what a "template part" is.

@noahtallen
Copy link
Member

Part of the problem is that users simply don't use the block navigator or block breadcrumbs. Most folks I've seen use template parts (including myself) have no clue what they are working with in the editor even though these tools exist! Maybe part of the solution is making those easier to access, but how? The breadcrumb is as obvious as it can be at the bottom, and the block navigator is not visible while you are editing anyways, so it isn't really a tool for "reorienting myself immediately."

Beyond this, I worry that our focus on removing stuff from the canvas is leading to a UX which is harder to understand and reason about. I think layout building requires tools, organization, and an understanding of how stuff works on your site. Part of that would include outlines and tools in-line while you are working with different site-level documents.

This is very different from post writing, which is all about removing distractions to focus on writing text. These are entirely different use cases, and other applications for those use cases reflect that. (e.g. MS word, google docs have little inline UI, whereas page builders and site builders often do have outlines and text tips to keep you oriented.)

In my mind, the site editor is less about writing content, and much more about designing a great site!

In this case, the label would obscure other elements

Isn't this ok given that it would only show up while specifically editing the template part?

the important consequence of that behaviour is the impact it will have on the rest of the site when changes are saved

I think this is very true. But I don't think this PR specifically solves that problem. (Though I totally want to dig into solving that problem as well since it affects so many different blocks!)

The problem we have currently is that even with tools like the block navigator, it takes a lot of effort to determine exactly which blocks on the canvas are children of the template part block. Currently, without a border/label while editing, you would have to click on every block until you find the location where it switches to a different block being the parent. That's a pretty rough UX. I would even say that if you don't understand what a template part is, you wouldn't even know to try that approach! (And it would certainly not be familiar to users who don't use the block navigator or breadcrumb very often.)

The border solves part of that: while editing the template part, you know exactly where it begins and ends. I would suggest that the label solves the other part: exactly "what" does this border indicate? I think this is intuitive if you know what a template part is, and even if you don't, gives you enough information to work with it.

I think exposing the current template part in the top bar next to the document title ( Homepage • Footer ) might be better overall

I think the big problem with this is that it relies on users to know to go looking at the header to find the information, similar to the breadcrumb or block navigator.

I want to mention again that we are very open to more designs and explorations around this! The thing is: the experience isn't great right now, and I think something (like this PR along with the border) is better than nothing. I should also mention that I'm thinking about these problems from the standpoint of a beginner to WordPress (and even site building in general), so I'm very much in favor of being obvious about what's happening.

@vindl
Copy link
Member

vindl commented Aug 26, 2020

I might even go one step further, and question why we're adding this at all. There are already two locations where the template part is exposed to users; the breadcrumb, and the block navigator. Do we need another one? If so, what problem is it solving?

Based on some previous discussion I'm not sure if the breadcrumb is going to stick around in the long run, but even if it does it doesn't help much from my experience. It's not easy to tell at a glance if you are editing something in the header, especially when there is a lot of nesting and that line gets too long.

I think the problem with the block navigator, aside from discoverability that @noahtallen already mentioned, is that I'd have to toggle it back on whenever I want to make sure that I'm actually adding a block to the template part, and not to some other area. This will be cumbersome when adding multiple blocks.

@dubielzyk
Copy link

Do you think something like a spotlight-like UI would work better, @jameskoster @shaunandrews ? Or is it the notion of adding any UI to the canvas at all?
image

@jameskoster
Copy link
Contributor

I think this is very true. But I don't think this PR specifically solves that problem. (Though I totally want to dig into solving that problem as well since it affects so many different blocks!)

You know what - you're right 😁 I think I'm getting too far ahead of myself, thinking about the holistic flow of editing template parts.

is that I'd have to toggle it back on whenever I want to make sure that I'm actually adding a block to the template part

Just to note, this may not always be the case: #22113

Do you think something like a spotlight-like UI would work better

It's an interesting idea, but forcing spotlight mode feels a little heavy-handed to me. To avoid disrupting a users edit flow I wonder if the consequences should be revealed when they're finished - on save - using the multi-entity-saving patterns that are being worked on?

@jeyip jeyip changed the title Template Part Block: Add a block label [Prototype] Template Part Block: Add a block label Aug 31, 2020
@jeyip
Copy link
Contributor Author

jeyip commented Aug 31, 2020

Not quite sure where we landed with this PR @jameskoster @shaunandrews. Because of the experimental nature of the site editor (and the ability to easily revert changes), @noahtallen @Addison-Stavlo and I discussed the idea of merging this PR.

It would be a concrete change we could analyze and test in the context of other template part editing improvements, and if folks are still uncertain about it's utility in the future, we could roll back the updates made here.

@shaunandrews
Copy link
Contributor

Not quite sure where we landed with this PR

I'm not a fan, but I'm also not sure I have veto power :)

--

image

I'm still seeing some bleed-through of my theme's styles in the font, which is pretty small. This seems to be using a new icon I haven't seen before, and its pretty hard to recognize at this small size.

image

In Top Toolbar mode, it can be difficult to understand what block is selected; I see the "Header" label and a subtle border around the Template Part block — but I have have the Site Title block selected.

@noahtallen
Copy link
Member

Do you think there is a better way to solve this problem?

@jameskoster
Copy link
Contributor

In Top Toolbar mode, it can be difficult to understand what block is selected; I see the "Header" label and a subtle border around the Template Part block — but I have have the Site Title block selected.

This is a very good point. To the untrained eye it would be easy to assume that the header template part is selected there.

Do you think there is a better way to solve this problem?

Perhaps we should concentrate efforts around the persistent block navigator seeing as that has the potential to solve this issue, and provide affordances for various other template editing activities as well?

@davipontesblog
Copy link

Do you think something like a spotlight-like UI would work better

I am a firm believer that a spotlight-like UI like this improves the understanding of template parts, and what exactly you are editing in the Site Editor. Not only it provides better focus (imagine a homepage layout from our current Gutenboarding designs in the site editor, with tens of blocks competing for attention, in their full colorful glory), but it allows users to learn their site structure as they edit.

@jeyip
Copy link
Contributor Author

jeyip commented Sep 1, 2020

I see the "Header" label and a subtle border around the Template Part block — but I have the Site Title block selected.

This is a very good point. To the untrained eye it would be easy to assume that the header template part is selected there.

This was deliberately introduced to provide context about which template part our blocks exist in.

Template Part Block Selected Template Part Child Block Selected
Screen Shot 2020-08-12 at 2 18 35 PM Screen Shot 2020-08-12 at 2 18 42 PM

Perhaps we should concentrate efforts around the persistent block navigator seeing as that has the potential to solve this issue, and provide affordances for various other template editing activities as well?

This is an interesting idea. For those with less context, here's a link to the persistent block navigator issue. I'm curious about a few things:

  1. It seems like finding the persistent block navigator would be less obvious to the user, it might be less intuitive than labels (because of visual proximity to what the user is editing), and it would take up more visual real estate. I'm sure there are benefits, and I was hoping that either @jameskoster or @shaunandrews could elaborate. I'd love to learn more.
  2. What are the other affordances for template editing activities you had in mind @jameskoster ?

@Addison-Stavlo
Copy link
Contributor

Perhaps we should concentrate efforts around the persistent block navigator seeing as that has the potential to solve this issue, and provide affordances for various other template editing activities as well?

From what I can tell this does aim to solve the same sort of issue, but from a different use case and perspective. That block navigator will add some great improvements, but I don't see it making the need for on-page visual indicators to go away entirely.

I am a firm believer that a spotlight-like UI like this improves the understanding of template parts, and what exactly you are editing in the Site Editor. Not only it provides better focus (imagine a homepage layout from our current Gutenboarding designs in the site editor, with tens of blocks competing for attention, in their full colorful glory), but it allows users to learn their site structure as they edit.

1000% agree with this. Casting shade and/or dimming opacity on elements outside of the template part when it is selected feels very fitting for conveying that it is a separate entity from its surroundings.

@vindl
Copy link
Member

vindl commented Sep 1, 2020

Perhaps we should concentrate efforts around the persistent block navigator seeing as that has the potential to solve this issue, and provide affordances for various other template editing activities as well?

@jameskoster How will this interact with proposed sidebar navigation in #23939? 🤔

@shaunandrews
Copy link
Contributor

This was deliberately introduced to provide context about which template part our blocks exist in.

I imagine this won't be an issue if we add the current template part name next to the template name in #20877

@noahtallen
Copy link
Member

I imagine this won't be an issue if we add the current template part name next to the template name in #20877

I mean, this is very similar to the block breadcrumb, and lots of people don't know about that even though it is visible all the time :) (But I do agree that it would be a nice improvement!)

@jeyip
Copy link
Contributor Author

jeyip commented Sep 1, 2020

I mean, this is very similar to the block breadcrumb, and lots of people don't know about that even though it is visible all the time :) (But I do agree that it would be a nice improvement!)

The template part name in the topbar also reminds me of breadcrumbs. I think that it's an improvement as well, but I think Addie made a great observation.

From what I can tell this does aim to solve the same sort of issue, but from a different use case and perspective. That block navigator will add some great improvements, but I don't see it making the need for on-page visual indicators to go away entirely.

Maybe both would be warranted? The on-page visual indicators seem like they could complement the template part name in the topbar.

@jameskoster
Copy link
Contributor

jameskoster commented Sep 1, 2020

It seems like finding the persistent block navigator would be less obvious to the user

With all the nesting involved, I don't think it would be unreasonable to consider initially auto-exposing the block navigator when template editing.

and it would take up more visual real estate.

For just learning what template part you're editing at a glance I agree it is overkill. But that is what the breadcrumb (or template part name in the top bar :)) is for.

What are the other affordances for template editing activities you had in mind @jameskoster ?

In particular; quickly and accurately selecting heavily nested blocks, dragging/dropping between those nested groups, and adding blocks in the correct location in one click. I'm sure there are others to be explored :)

@MichaelArestad
Copy link
Contributor

I mean, this is very similar to the block breadcrumb, and lots of people don't know about that even though it is visible all the time :) (But I do agree that it would be a nice improvement!)

@noahtallen I wouldn't compare it to the success of breadcrumbs. The location of the breadcrumbs make them near invisible. Putting the name in the top bar would be a visible change that could reduce the need for this PR.

I also strongly feel a persistent block navigator will be key to helping folks navigate the interface. It's something I'm particularly looking forward to. It also will alleviate this issue.

@kellychoffman
Copy link
Contributor

It seems like we're at a bit of impasse. The original problem still stands:

When the child of a template part block is selected, it can be unclear which template part we're modifying.

But this particular solution introduces an additional problem, which in my mind blocks the merging of this PR. I think its worth pausing this particular PR and going back to the drawing board to explore more options for solving this problem.

@dubielzyk
Copy link

Agreed. Looks like this isn't the right way to go.

In the original issue I've suggested (#22064 (comment)) we try out a spotlight like mode for template parts (no label or borders). Perhaps we can try that out and see what people think?

@noahtallen
Copy link
Member

I think its worth pausing this particular PR and going back to the drawing board to explore more options for solving this problem.

I agree with this since we are now at an impasse :) I do still feel this is one of the best options if the only worry is helping folks understand template parts, but I'll certainly concede it causes some other problems along the way. I look forward to hearing more ideas about how to solve it!

@jeyip
Copy link
Contributor Author

jeyip commented Sep 2, 2020

Thanks for the input everyone! 🙏

I appreciate the honest and open discussion. I'll go ahead and close this PR for now. If things change, we can always revisit this idea in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Design Feedback Needs general design feedback. [Status] In Progress Tracking issues with work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.